Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Precompute encodeWithKeys buffer size to avoid resizes #1269

Conversation

howardjohn
Copy link
Contributor

This is strictly an optimization. Right now we always undersize the buffer then later increase it.
This changes the code to accurately size it the first time to ensure we never re-allocate.

name                old time/op    new time/op    delta
RecordReqCommand-6    2.26µs ± 5%    2.10µs ± 5%   -7.39%  (p=0.000 n=10+10)
RecordViaStats-6      2.70µs ± 5%    2.53µs ± 4%   -6.31%  (p=0.000 n=10+10)

name                old alloc/op   new alloc/op   delta
RecordReqCommand-6      426B ± 0%      384B ± 0%   -9.86%  (p=0.000 n=10+10)
RecordViaStats-6        594B ± 0%      552B ± 0%   -7.07%  (p=0.000 n=10+10)

name                old allocs/op  new allocs/op  delta
RecordReqCommand-6      25.0 ± 0%      17.0 ± 0%  -32.00%  (p=0.000 n=10+10)
RecordViaStats-6        28.0 ± 0%      20.0 ± 0%  -28.57%  (p=0.000 n=10+10)

See #1265

@howardjohn howardjohn requested review from rghetia and a team as code owners September 14, 2021 17:27
@google-cla google-cla bot added the cla: yes label Sep 14, 2021
@howardjohn howardjohn force-pushed the encodeWithKeys/precompute-length branch from 0009364 to 7689b28 Compare September 14, 2021 19:55
@howardjohn
Copy link
Contributor Author

I don't think the test failure is from my test. Maybe its a flaky test? Its in the tracing code, not stats.

@@ -59,8 +59,15 @@ func (c *collector) clearRows() {
// encodeWithKeys encodes the map by using values
// only associated with the keys provided.
func encodeWithKeys(m *tag.Map, keys []tag.Key) []byte {
// Compute the buffer length we will need ahead of time to avoid resizing latter
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/latter/later

@dashpole
Copy link
Collaborator

Well, fix the typo, and hopefully it doesn't flake this time :)

This is strictly an optimization. Right now we always undersize the buffer then later increase it.
This changes the code to accurately size it the first time to ensure we never re-allocate.

```
name                old time/op    new time/op    delta
RecordReqCommand-6    2.26µs ± 5%    2.10µs ± 5%   -7.39%  (p=0.000 n=10+10)
RecordViaStats-6      2.70µs ± 5%    2.53µs ± 4%   -6.31%  (p=0.000 n=10+10)

name                old alloc/op   new alloc/op   delta
RecordReqCommand-6      426B ± 0%      384B ± 0%   -9.86%  (p=0.000 n=10+10)
RecordViaStats-6        594B ± 0%      552B ± 0%   -7.07%  (p=0.000 n=10+10)

name                old allocs/op  new allocs/op  delta
RecordReqCommand-6      25.0 ± 0%      17.0 ± 0%  -32.00%  (p=0.000 n=10+10)
RecordViaStats-6        28.0 ± 0%      20.0 ± 0%  -28.57%  (p=0.000 n=10+10)
```
@howardjohn howardjohn force-pushed the encodeWithKeys/precompute-length branch from 7689b28 to f8d6079 Compare September 14, 2021 21:28
@dashpole dashpole merged commit ad0b46e into census-instrumentation:master Sep 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants